SPE: reply-paced LCD polling and a calmer freshness gate — no more mirror blink - #5542

Reviewed against the head checkout at bad920de. CI is green on all five checks (build, check-macos, check-windows, Static checks, Sanitizer option configures) — noted, not relied on: none of them exercise SPE cadence.

Findings below are reasoned from code, not reproduced at runtime — I have no build, no amplifier, and no ser2net proxy here.

1. Issue fit

Partially. #5541 names two causes and this PR addresses both in the right places: the freshness gate becomes an absolute 1.8 s window decoupled from the poll gap, and SpeApplet::setLcdFresh dims via SpeLcdWidget::setStale instead of clear()ing, with the hard clears correctly retained at setConnected(false)/clearTelemetry() (SpeApplet.cpp:665-666) and at the presentation switch. I walked the disconnect and the docked⇄floating paths and both do reach the idle glass, so the body's "returns to the idle glass only when truly obsolete" claim holds.

Cause 1 — the phase-lock — is where I could not make the code match the claim. The triage on #5541 specified "re-arm m_lcdTimer from the display reply and make it single-shot … needs a fallback re-arm on the timeout path too." This PR adds the reply re-arm but leaves m_lcdTimer a repeating timer still armed inside requestLcdFrame() (SpeConnection.cpp:96) — setSingleShot is set for m_reconnectTimer, m_powerOnTimer and m_lcdStaleTimer, never for m_lcdTimer. That solves the lost-reply retry the triage worried about, but it means the design's central invariant is not actually implemented. See Blocker 1.

2. Scope

Everything in the diff is explained by #5541. No CHANGELOG.md entry (correct). No settings, protocol verb, capability or public-API surface added. No guard or confirmation removed — the key gate on m_lcdFresh in updateCommandsEnabled() (SpeApplet.cpp:653) is untouched, which is the important negative: dimming instead of blanking does not loosen the TX-adjacent front-panel key gating.

3. Blockers

1. The "a second request is never in flight before the previous reply has fully arrived" invariant is not implemented, and it fails on exactly the slow link the doc uses to justify the 250 ms gap. (inline: docs/…/spe-expert-amplifier-design.md:323, src/core/SpeConnection.cpp:36)

m_lcdTimer is repeating, interval 250 ms, and requestLcdFrame() arms it at send time (SpeConnection.cpp:96). The reply re-arm only wins the race when the reply decodes within 250 ms of the request. When round-trip ≥ 250 ms the send-time timer fires first, issues a second request while the first reply is still arriving, and — because requestLcdFrame() restarts the timer — the loop free-runs at a fixed 250 ms send-to-send cadence with none of the amp's latency folded in.

That is the original bug's precondition, not its cure: a fixed cadence commensurate with the 100 ms status poll (LCM 500 ms, so every second display request lands on a status-poll boundary). And it lands precisely where the doc says the design degrades gracefully — the doc's own arithmetic puts a 371-byte display frame at ~390 ms on a 9600 baud proxy serial side, i.e. well past 250 ms. At 19200 the doc's own ~450 ms cadence implies ~200 ms of reply latency, leaving ~50 ms of margin against an amp response latency the same doc calls variable.

The old 600 ms free-run needed a 600 ms round trip to overlap; this needs 250 ms. The margin shrank 2.4× while the doc's claim got stronger.

Two ways out, both small — a maintainer should pick:

• What the triage specified: m_lcdTimer.setSingleShot(true), re-arm only from the reply, and add the explicit timeout-path re-arm so a lost reply still retries (a separate, longer retry interval rather than reusing the 250 ms gap).
• Or keep the repeating timer purely as a watchdog with an interval above worst-case RTT, and gate requestLcdFrame() on an m_lcdRequestOutstanding flag cleared by the display callback.

Either way the doc sentence needs to state the condition (round trip < the gap) rather than assert the invariant unconditionally.

4. Nits (non-blocking)

• The raised collision probability is unaddressed and the triage asked for a decision on it. #5541's triage flagged that 600→250 ms raises display traffic ~2.4× and therefore raises per-status-poll collision probability even once bursts can't form, and proposed pollTick() skipping a status request while a display reply is outstanding (with care that a skipped tick must not count toward m_silentPolls). The PR doesn't do it and the body doesn't say why not. Reasonable to defer — worth saying so explicitly. Needs maintainer decision.
• m_lcdTimer.start() in the display callback is unguarded (SpeConnection.cpp:36) where requestLcdFrame() checks m_lcdWanted && m_connected. If anything on the lcdFrameReceived chain ever tore the connection down, the timer would be re-armed after teardown stopped it and would then tick forever into an early return. I traced the current chain (MainWindow_Wiring.cpp:6496 → SpeApplet::setLcdFrame → m_lcd->setFrame) and it is not reachable today; the adjacent pre-existing m_lcdStaleTimer.start() has the same shape. Cheap to make both conditional.
• The veil alpha 170 is an unmotivated magic number and not a token (SpeLcdWidget.cpp:148). It leaves ~33 % of the glass's original fg/bg contrast, which is the whole point — but "keeps the last screen for context" and "legible" are different bars, and docs/a11y.md asks 3:1 for non-text state distinctions. Concrete values are explicitly allowed as an intermediate there, with a follow-up issue for the token; worth filing one.
• No test, and half the change does have a deterministic seam. SpeLcdWidget is a plain QWidget: setFrame → setStale(true) → clear() retaining/dropping m_hasFrame is assertable socket-free, and the triage on #5541 asked for exactly that plus a lost-reply re-arm check. The scheduler half genuinely has no injection seam (SpeConnection owns its QTcpSocket and requestLcdFrame() early-returns on !m_connected), and I would not ask for a fake amplifier peer to get one. Calling this a nit rather than a blocker because the primary reported symptom's root cause sits on the untestable side — a maintainer could reasonably require the widget-side test.
• #5541 carries maintainer-review ("Requires maintainer review before any action is taken") and this PR was opened a minute after the issue, before any ruling. Procedural, not a code problem.

5. What I tried to break (and failed)

• Stale-state leakage across reconnect. Chased whether m_stale can survive into a live frame: setFrame() clears it (SpeLcdWidget.cpp:57) and clear() clears it, so a fresh frame is never veiled. Held.
• clear()'s new early-return condition. if (!m_hasFrame && !m_stale) return; — I looked for the state m_stale == true, m_hasFrame == false (reachable: setResponding(false) clears the widget, then the 1.8 s stale timer fires and calls setStale(true)). It paints correctly (no veil branch, "waiting for display…" branch taken) and the next clear() still resets both. Held.
• Painter state under the veil. p.setPen(Qt::NoPen) is set before the bezel and never changed before drawRect, so the veil has no stray 1px stroke, and the veil/!m_hasFrame branches are mutually exclusive. Held.
• Whether dimming loosens the front-panel key gate. updateCommandsEnabled() still gates on m_lcdFresh (SpeApplet.cpp:653) and setLcdFresh still assigns it before dimming. The keys go dead on staleness exactly as before; only the glass changed. Held — this was the one I most expected to find a hole in.
• The docked⇄floating clear() ordering. m_lcd->clear() runs before setLcdFresh(false), so the subsequent setStale(true) lands on an already-blank widget rather than veiling a previous session's image. Correct as written.
• Could not check: anything runtime — the phase-lock claim, whether 1.8 s is the right window on the reporter's link, and whether the veil is legible on a real 1.5K-FA glass. The reporter's ser2net serial-side baud is not stated in #5541, which is what would settle whether Blocker 1 bites their station specifically; worth asking them.

6. Recommendation

Request changes, on Blocker 1 only. The freshness-gate and dim-don't-blank halves are well-reasoned, correctly scoped, and leave the safety-relevant key gating untouched — I tried several ways to break them and could not. But the reply-pacing half ships the doc's strongest claim without the code that makes it true, and the 250 ms gap is only defensible because of that claim; on a slow proxy serial side it degrades into a fixed-cadence free-run at 2.4× the old request rate, which is the reported bug with more traffic behind it.

Concrete next step: make m_lcdTimer single-shot with an explicit longer-interval retry on the lost-reply path (or add the in-flight guard), and condition the doc sentence on round-trip < gap. Then the maintainer decision on pollTick() skipping during an outstanding display reply can be taken on its own merits rather than as a load-bearing assumption.

Nice work on the root-cause writeup — the phase-lock explanation is the kind of thing that usually gets papered over with a bigger timeout.

_{🤖 aethersdr-agent · cost: $7.2874 · model: claude-opus-5}